Skip to content

Conversation

@r-sharp
Copy link
Contributor

@r-sharp r-sharp commented Jan 30, 2026

… ToDo plugin work nicely.

PR Summary

Sci/Tech Reviewer: @Pierre-siddall
Code Reviewer: @ericaneininger

Some frustrating changes to formatting/syntax in order to get VSCode's internal linters to stop whinging and the ToDo plugin to correctly identify the things to do.

Code Quality Checklist

  • I have performed a self-review of my own code
  • My code follows the project's style guidelines
  • Comments have been included that aid understanding and enhance the readability of the code
  • My changes generate no new warnings
  • All automated checks in the CI pipeline have completed successfully

Testing

  • This change has been tested appropriately (please describe)
    i ran it - it did what I expected..
    I ran pytest - all the tests passed.

Security Considerations

  • I have reviewed my changes for potential security issues
  • Sensitive data is properly handled (if applicable)
  • Authentication and authorisation are properly implemented (if applicable)

AI Assistance and Attribution

  • Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

VSCode has GitHub copilot enabled for the autocomplete / suggestions to speed up writing the code.

Sci/Tech Review

  • I understand this area of code and the changes being added
  • The proposed changes correspond to the pull request description
  • Documentation is sufficient (do documentation papers need updating)
  • Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

  • All dependencies have been resolved
  • Related Issues have been properly linked and addressed
  • Code quality standards have been met
  • Tests are adequate and have passed
  • Security considerations have been addressed
  • Performance impact is acceptable

Copy link
Contributor

@Pierre-siddall Pierre-siddall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Roddy, this basically is ready to go as all the functionality would work and these changes are mainly edits to comply with VScode's line length checking. I only have one tiny query about reducing the overhead of umdp3 checker when one of the constructors is called with a nested data structure. See if you agree with making a trade off between robustness and performance. Otherwise I'm happy for this to head onto trunk :) .

@Pierre-siddall Pierre-siddall self-requested a review February 2, 2026 15:17
Copy link
Contributor

@Pierre-siddall Pierre-siddall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a mistake on my last set of comments this looks good and functional to me, good job Roddy, on to @ericaneininger for CR.

@r-sharp
Copy link
Contributor Author

r-sharp commented Feb 2, 2026

I really don't find GitHub's way of presenting info about what's happening at all intuitive.
It took me a while to work out where Yash had commented, as I couldn't see anything (resolved comments are an absolute PITA)

@Pierre-siddall : Apologies, I've mistakenly added yet another commit to my Branch.
This was changes I was making to get the script to work when called from rose-stem.
In essence, it was not getting the paths to the files, so ther's a new loop to add them and then turn the 'strings' back into Paths. It proibably could have been done more neatly, but it's a discrete change that works.
I've also removed some attributes to classes that had become redundant over time, and another 'ToDo' note to myself about how badly named everything is. A Conformance checker contains a list of 'active checkers' each active checker contains a collection of checker functions, and I've used the term 'checker' to describe all 3 because I'm a bear of very little brain and long words confuse me.

And this is why I feel the "structure" of the code needs someone with a better feel for objects/classes and how to use them to look at the 'layout' and ask "Why have you done that ?, you could have just "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants